-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bottom up iterator #107
base: master
Are you sure you want to change the base?
Bottom up iterator #107
Conversation
… into bottom-up-iterator
… into bottom-up-iterator
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #107 +/- ##
===========================================
- Coverage 67.35% 32.59% -34.76%
===========================================
Files 21 24 +3
Lines 729 908 +179
===========================================
- Hits 491 296 -195
- Misses 238 612 +374 ☔ View full report in Codecov by Sentry. |
src/bottom_up_iterator.jl
Outdated
current_programs::Queue{RuleNode} | ||
mutable struct BottomUpState | ||
bank::Any | ||
data::Any |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-urgent thing to think about: should we leave bank
and data
as Any
, or should we introduce some subtyping? I assume the bank will always be either a Dict
so that programs are ordered over their type, or some kind of priority as in Brute. If we figure out more precise type, the code might be faster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have decided to introduce two new abstract types: BUDepthBank
and BUDepthData
that the concrete iterators will need to extend. Please let me know what you think about this.
src/bottom_up_iterator.jl
Outdated
iter::DepthIterator | ||
)::Dict{Symbol, Vector{RuleNode}} | ||
grammar::ContextSensitiveGrammar = get_grammar(iter.solver) | ||
bank::Dict{Symbol, Vector{RuleNode}} = Dict{Symbol, Vector{RuleNode}}() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it make sense to make the bank type Dict{Symbol, Dict{Int,Vector{RuleNode}}}
? so that you can also index it over depth for easier fetching?
Julia 1.12 will clean up when implicit world age increments happen. In particular, there will no longer be implicit increments within the same statement at toplevel. However, we are likely retaining implicit increments before each statement within @testset. This PR makes a small rearrangements to be compatible with this change. See JuliaLang/julia#56509.
@TudorMagirescu: |
Add abstract layer to iterator hierarchy
Small tweak for Julia 1.12 world age change
Add `Aqua.jl` to test for this in the future as well as other Aqua tests. Ignoring the type piracies on `RuleNode`s and `AbstractGrammar`. Ideally, this functionality would be moved to `HerbCore`, but that can wait.
Remove `FixedShapedIterator`
…-16-01-46-08-093-02170095097 CompatHelper: bump compat for HerbSpecification to 0.2, (keep existing compat)
Simplify compat for `Random`
The caret is the default SemVer behavior (https://pkgdocs.julialang.org/v1/compatibility/\#Caret-specifiers) and it seems other projects leave it off.
…-13-01-52-34-511-01072321669 CompatHelper: bump compat for HerbGrammar to 0.5
I have been going over our struggles to define a clean interface for the bottom-up search. The situation with two iterators is quite clunky, and avoiding the inner iterator overloads the memory. We have to make it easy for people to implement various versions rather quickly. Here is a proposal for a generic bottom-up iterator (the capitalized functions are the interface):
The interface functions do the following:
This interface would allow us to implement many versions of bottom-up search easily:
Optimisations are also easy to do:
Issue: how to handle returning individual programs when manipulating uniform trees within the iterator |
Just to trigger CI for now...